Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add kaggle user agent to gcp integration clients. #671

Merged
merged 1 commit into from
Dec 9, 2019
Merged

Conversation

vimota
Copy link
Contributor

@vimota vimota commented Dec 7, 2019

No description provided.

@vimota vimota requested review from rosbo and mcollins42 December 7, 2019 08:37
@vimota
Copy link
Contributor Author

vimota commented Dec 7, 2019

This works for all integrations except the automl TablesClient, they have a bug in their client initialization where if you provide a client_info= parameter it throws TypeError: type object got multiple values for keyword argument 'client_info'.

Cause of it is this line, where they get a client_info from kwargs, but then pass it as a parameter again, so it's passed in twice (as a named parameter and in the original kwargs):
https://github.com/googleapis/google-cloud-python/blob/9e5ca9a6e468d246e3aab5392ef0eeb4e802ec3d/automl/google/cloud/automl_v1beta1/tables/tables_client.py#L109

This should be easy to fix, I'll sent them an PR/Issue for it.

@vimota vimota marked this pull request as ready for review December 7, 2019 08:38
@rosbo
Copy link
Contributor

rosbo commented Dec 7, 2019

Note, the master build is failing and is likely the cause of this failure. I will try fixing it today. Once master is green, you will have to rebase from master to get all the latest changes and make sure the build is green. Thanks

@rosbo
Copy link
Contributor

rosbo commented Dec 7, 2019

I have fixed the master build. It should be good now.

Copy link
Contributor

@mcollins42 mcollins42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!


# TODO(vimota): Remove the exclusion of TablesClient once
# the client has fixed the error:
# `multiple values for keyword argument 'client_info'``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that an actual error that you see if this is set? I was hoping that Python would just ignore the kwargs value if a named value was already passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@vimota
Copy link
Contributor Author

vimota commented Dec 9, 2019

I have fixed the master build. It should be good now.

Thanks, rebased and rerunning!

@vimota vimota merged commit f9b559a into master Dec 9, 2019
@vimota vimota deleted the useragent branch December 9, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants